Conversation
| You can also use this method to check for view existence using 'try catalog.load_view() except NoSuchViewError'. | ||
| Note: This method doesn't scan data stored in the view. |
There was a problem hiding this comment.
Nit: I would say we remove these.
There was a problem hiding this comment.
+1 this is simply just load a view it doesn't have to follow the convention of load_table.
| assert actual == expected | ||
|
|
||
|
|
||
| def test_load_view_404(rest_mock: Mocker) -> None: |
There was a problem hiding this comment.
Would also add a test that should return a view does not exists when trying to load a table with loadView
There was a problem hiding this comment.
Correct me if I'm wrong, but isn't this case already covered with the test_load_view_404 test?
If no view with the same identifier as the table identifier is found in the warehouse, this would be the same case as loading a non existent view.
There was a problem hiding this comment.
This is implementation dependent on the server, so this is an edge case to return the correct exception
But the test is better placed in integration tests rather than mocking
Would be cool to also add integration tests to the test_catalog.py
| You can also use this method to check for view existence using 'try catalog.load_view() except NoSuchViewError'. | ||
| Note: This method doesn't scan data stored in the view. |
There was a problem hiding this comment.
+1 this is simply just load a view it doesn't have to follow the convention of load_table.
| self._check_endpoint(Capability.V1_LOAD_VIEW) | ||
| response = self._session.get( | ||
| self.url(Endpoints.load_view, prefixed=True, **self._split_identifier_for_path(identifier, IdentifierKind.VIEW)), | ||
| params={}, |
There was a problem hiding this comment.
nitty: we don't need to pass in the empty params
|
I think we just need a rebase here, and I agree with @gabeiglio on adding an integ test if tck has the view support. |
3 participants
Rationale for this change
This is part of #818 and implements the
load_viewmethod for REST catalogs.Are these changes tested?
Unit tests are added for:
Are there any user-facing changes?
This adds the
load_viewmethod for REST catalogs